fix: receive message type assumption on nullish values#1366
fix: receive message type assumption on nullish values#1366bbortt merged 1 commit intocitrusframework:mainfrom
Conversation
bbortt
added
Type: Bug
Prio: High
State: Review
core/citrus-base/src/main/java/org/citrusframework/actions/ReceiveMessageAction.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| var payload = message.getPayload(String.class); | ||
| var payload = isNull(message.getPayload()) && "null".equals(message.getPayload(String.class)) ? null : message.getPayload(String.class); |
There was a problem hiding this comment.
Can't this be done better? It is smelly. Having a "null" string here seems to be the issue. Can't this be solved earlier? My feeling is that message.getPayload(String.class) should never return "null" unless there is a payload which really resolves to "null"
There was a problem hiding this comment.
I think the problem was something along the line of this: a JSON message with null'ish payload resolves to {} when calling Message#getPayload(String) - but of course it does not have an Object representation, because it is an empty object. it is not "effectively nullish". thus a check on Message#getPayload() is not sufficient. it is additionally required to make sure the String representation is also "null".
I've refactored the code a bit, moved this into a method with a comment. that should make it more clear.
I don't think it can be catched earlier on, respectively this would go to deep and change too much of "accepted behavior".
core/citrus-base/src/main/java/org/citrusframework/actions/ReceiveMessageAction.java
Show resolved
Hide resolved
core/citrus-base/src/main/java/org/citrusframework/actions/ReceiveMessageAction.java
Outdated
Show resolved
Hide resolved
core/citrus-base/src/main/java/org/citrusframework/actions/ReceiveMessageAction.java
Show resolved
Hide resolved
core/citrus-base/src/main/java/org/citrusframework/actions/ReceiveMessageAction.java
Outdated
Show resolved
Hide resolved
a too eager message type assumption using `getPayload(String)` on the message leads to `null`'is values not being thus. e.g. it's interpreted as `XML` instead of `PLAINTEXT`. occured during implementation of citrusframework/citrus-simulator#315.
bbortt
force-pushed
the
fix/message-type-assumption
branch
from
186e281 to
b03604d
Compare
|
would it help to set the default message type to PLAINTEXT? I think XML as a default is a historic value and as you have mentioned there is always a default validator for PLAINTEXT available. If it helps to simplify the checks we can change the default to PLAINTEXT. Otherwise I am also good with the solution provided as it is right now. Many thanks! |
|
phu, we've tried (@tschlat and me) but it goes quit deep.. I fell like that change would be too risky. thanks for the review! |
3 participants
a too eager message type assumption using
getPayload(String)on the message leads tonull'is values not being thus. e.g. it's interpreted asXMLinstead ofPLAINTEXT. occured during implementation of citrusframework/citrus-simulator#315.